Skip to content

[Rule Request]: Prohibiting use of aria-hidden and role='presentation' on focusable elements. #1169

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

mauryapari
Copy link
Contributor

@mauryapari
Copy link
Contributor Author

@kddnewton , @vhoyer. Could you please reiview this PR? Any feedback you provide would be greatly appreciated.

@vhoyer
Copy link
Collaborator

vhoyer commented Jul 15, 2024

Hey, could you add a new line at the end of the files that do not have them? github adds a little 🚫 at the end of the file when that's the case

Copy link
Collaborator

@vhoyer vhoyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code wise it seems fine, I just don't like nested ifs if I can avoid it, but not a real problem

@vhoyer
Copy link
Collaborator

vhoyer commented Jul 15, 2024

but prettier is complaining about your formatting, can you pls fix the problems before we merge it?

@mauryapari
Copy link
Contributor Author

Hey @vhoyer. fixed both of them. do check it.

@vhoyer
Copy link
Collaborator

vhoyer commented Jul 15, 2024

the docs both seem to still have the final EOL problem

@mauryapari
Copy link
Contributor Author

Hey. not sure what happened earlier. i have added it again for those.

@vhoyer
Copy link
Collaborator

vhoyer commented Jul 15, 2024

 $ /home/runner/work/eslint-plugin-vuejs-accessibility/eslint-plugin-vuejs-accessibility/node_modules/.bin/prettier --check '**/*'
Checking formatting...
[warn] docs/rules/no-aria-hidden-on-focusable.md
[warn] docs/rules/no-role-presentation-on-focusable.md
[warn] src/rules/__tests__/no-aria-hidden-on-focusable.test.ts
[warn] src/rules/__tests__/no-role-presentation-on-focusable.test.ts
[warn] src/rules/no-aria-hidden-on-focusable.ts
[warn] src/rules/no-role-presentation-on-focusable.ts
[warn] src/utils/hasFocusableElement.ts
[warn] Code style issues found in 7 files. Run Prettier with --write to fix.```

@mauryapari
Copy link
Contributor Author

okay i believe it should be resolved now. have a look.

@@ -0,0 +1,39 @@
import type { Rule } from "eslint";
// import type { AST } from "vue-eslint-parser";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be better if we removed the comments, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@vhoyer vhoyer merged commit 58c9e95 into vue-a11y:main Jul 17, 2024
2 checks passed
vhoyer pushed a commit that referenced this pull request Jul 17, 2024
…tion' on focusable elements. (#1169)

* Added rule for prohibiting use of aria-hidden and role=presentaion on focusable elements

* Refactored code into seprate files and utility file

* Added EOL for files with prettier errors

* Removed nested ifs

* Fixed EOL errors

* Added more lines

* Fixed prettier issues

* Removed commented code
@vhoyer
Copy link
Collaborator

vhoyer commented Jul 17, 2024

published https://github.com/vue-a11y/eslint-plugin-vuejs-accessibility/releases/tag/v2.4.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Rule Request] Rule for prohibiting use of aria-hidden or role=presentation on focusable elements
2 participants